feat: SQDSDKS-7423 - Add event mapping functionality and new MPRoktEventMapper class#16
Conversation
…entMapper class - Introduced MPRoktEventMapper class for mapping Rokt events to mParticle events. - Added methods in MPKitRokt for handling events with identifiers and global events. - Updated podspec to include Swift source files and public header files.
thomson-t
requested review from
BrandonStalnaker,
Copilot,
jamesnrokt and
rmi22186
There was a problem hiding this comment.
Pull Request Overview
Adds a new event mapping layer between Rokt and mParticle and exposes handler methods, along with project and podspec updates to include Swift sources.
- Introduces
MPRoktEventMapperfor convertingRoktEventinstances toMPRoktEvent - Adds
eventsWithIdentifier:onEvent:andglobalEventsOnEvent:inMPKitRokt - Updates Xcode project settings and Podspec to register the new Swift files and headers
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mParticle-Rokt/MPRoktEventMapper.swift | New Swift utility for mapping Rokt events |
| mParticle-Rokt/MPRoktEventMapper.h | Obj-C header for the mapper class |
| mParticle-Rokt/MPKitRokt.m | Added identifier-based and global event APIs |
| mParticle-Rokt.xcodeproj/project.pbxproj | Registered Swift sources and headers in the project |
| mParticle-Rokt.podspec | Expanded source_files to include Swift and headers |
Comments suppressed due to low confidence (3)
mParticle-Rokt/MPKitRokt.m:300
- This newly added public method lacks a documentation comment. Please add a docblock explaining its purpose, parameters, and return value.
- (MPKitExecStatus *)eventsWithIdentifier:(NSString *)identifier onEvent:(void (^)(MPRoktEvent * _Nonnull))onEvent {
mParticle-Rokt/MPRoktEventMapper.swift:10
- The
mapEvent(_:)method covers many event cases but lacks unit tests. Consider adding tests to verify each mapping branch.
@objc public static func mapEvent(_ event: RoktEvent) -> MPRoktEvent? {
mParticle-Rokt/MPRoktEventMapper.h:10
- The method declaration uses '++' instead of a single '+', causing a syntax error. Change it to '+' for a valid class method declaration.
+ (nullable id<MPRoktEvent>)mapEvent:(RoktEvent *)event;
| isa = PBXSourcesBuildPhase; | ||
| buildActionMask = 2147483647; | ||
| files = ( | ||
| B3D778532E02845700D887A4 /* MPRoktEventMapper.swift in Sources */, |
There was a problem hiding this comment.
MPRoktEventMapper.swift is listed twice under PBXSourcesBuildPhase, leading to duplicate file references. Remove the extra entry.
| B3D778532E02845700D887A4 /* MPRoktEventMapper.swift in Sources */, |
| @@ -15,7 +15,8 @@ Pod::Spec.new do |s| | |||
| s.swift_version = '5.3' | |||
There was a problem hiding this comment.
The podspec declares Swift version '5.3' but the Xcode project is set to Swift 6.0. Align the podspec swift_version with the project to avoid compatibility issues.
| s.swift_version = '5.3' | |
| s.swift_version = '6.0' |
thomson-t
force-pushed
the
feat/SQDSDKS-7423-events-api-subscription
branch
from
11b791c to
2a38817
Compare
BrandonStalnaker
left a comment
BrandonStalnaker
left a comment
There was a problem hiding this comment.
The code looks good for now (I know you said you have more changes planned) but please make sure to test through an app that uses cocaopods.
| FOUNDATION_EXPORT double mParticle_RoktVersionNumber; | ||
| FOUNDATION_EXPORT const unsigned char mParticle_RoktVersionString[]; | ||
|
|
||
| #if defined(__has_include) && __has_include(<mParticle_Appboy/MPKitAppboy.h>) |
There was a problem hiding this comment.
Please test this in an app that uses cocaopods to integrate the frameworks. I know this isn't need when clients use SPM but it is for cocaopods clients
Instructions
developmentSummary
Testing Plan
Reference Issue (For mParticle employees only. Ignore if you are an outside contributor)